Skip to content

Json key val#2687

Merged
igaw merged 3 commits intolinux-nvme:masterfrom
ikegami-t:json-key-val
May 15, 2025
Merged

Json key val#2687
igaw merged 3 commits intolinux-nvme:masterfrom
ikegami-t:json-key-val

Conversation

@ikegami-t
Copy link
Copy Markdown
Contributor

No description provided.

@ikegami-t
Copy link
Copy Markdown
Contributor Author

This is to check the NSID and NS if created correctly by the TP as mentioned by the issue comment #2668 (comment).

@igaw
Copy link
Copy Markdown
Collaborator

igaw commented Feb 7, 2025

Sorry you lost me here. What's the purpose with these changes?

@ikegami-t
Copy link
Copy Markdown
Contributor Author

No problem. The purpose is to output the json nsid key and value as below then I think that the TP can check the NS if created correctly. Thank you.

tokunori@tokunori-desktop:~/nvme-cli$ nvme-build create-ns /dev/nvme0 -s -1 -c 1 -f 0 -o json
{
  "create-ns":"Success, created",
  "nsid":"1"
}

@ikegami-t
Copy link
Copy Markdown
Contributor Author

Rebased the commits and added the tests changes also to check the create-ns nsid value.

Comment thread nvme-print-json.c Outdated
json_output_object(r);
}

static void json_output_key_value(bool space, bool eol, const char *key, const char *val,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two arguments space and eol should not be exposed to the user here. First, having function with several arguments using bools are always hard to read, secondly, this leaks the output layer features to the users side.

Comment thread nvme.c Outdated
if (!(le16_to_cpu(ctrl->oacs) & NVME_CTRL_OACS_NS_MGMT))
nvme_show_key_value(true, false, name, "Success, %s",
!strcmp(cmd, "create") ? "created" :
!strcmp(cmd, "delete") ? "deleted" : "");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strcmp cmd seems a bit excessive, why not just write 'create command was sucessful' or something along the line. or just 'success'. nvme create-ns -> success should be good enough.

@igaw
Copy link
Copy Markdown
Collaborator

igaw commented Mar 17, 2025

The nightly builds are currently not running. I'd like to get them working again before merging this because of the last patch.

@ikegami-t
Copy link
Copy Markdown
Contributor Author

Fixed the changes as mentioned and also noted about the nightly builds situation. Thank you.

@igaw
Copy link
Copy Markdown
Collaborator

igaw commented Mar 18, 2025

FYI, @MaisenbacherD is updating/re-configuring the nightly CI runner environment. As all projects, it takes a bit longer than expected :)

@ikegami-t
Copy link
Copy Markdown
Contributor Author

Thank you. Noted it.

@MaisenbacherD
Copy link
Copy Markdown
Contributor

The nightly tests are running again :)

ikegami-t added 3 commits May 15, 2025 09:09
This is to output a command result as json key and value.

Signed-off-by: Tokunori Ikegami <[email protected]>
This is to use the command result NSID created by TP.

Signed-off-by: Tokunori Ikegami <[email protected]>
Also the command output-format is changed to json.

Signed-off-by: Tokunori Ikegami <[email protected]>
@igaw
Copy link
Copy Markdown
Collaborator

igaw commented May 15, 2025

I"ve rebased the series and refactored is_ns_mgmt_support slightly. Let's see what the nighly tests show.

@igaw igaw merged commit 26f413a into linux-nvme:master May 15, 2025
16 checks passed
@igaw
Copy link
Copy Markdown
Collaborator

igaw commented May 15, 2025

Nightly builds passed. So all good. Thanks everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants